No actionable comments were generated in the recent review. 🎉

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Internal action results are still dropped on this path.

I chose for internal actions to not produce their own results, and also not to propagate a prior result across them. (Why should they? Don't see good reason for them to, honestly.) It makes most sense IMO for only an action that produces a result itself, to be source of one.

Please lock down previousResult semantics across concurrent groups with waits

Flagged in the initial comment.

Consider making previousResult optional in the interface

Explicit is better than implicit. There's always a previous result value/variable. It's not actually optional semantically. (I even considered using const previousResult: VariableValue = undefined every place, for documentation, but didn't carry through on it.)

Nice chaining logic — consider avoiding in-place mutation of extras

I kind of prefer less object creation churn, but Julian's the one who will have a meaningful preference here, probably.

This will fix #4064

The idea is awesome and I 100% support it but my suggestion to your question

how to propagate a result to a subsequent concurrent action group that contains waits.

Why not use something like n8n does it: any node that's downstream, can access any upstream node's result using the node's name. For example, first node is called "Message", the next node can get the data directly with $json (in n8n) or $(this:result) with your implementation in Companion, but then next nodes or even the current one, can also get it with $('Message').item.json. This is an example that works elsewhere and is just an idea, but obviously completely out of scope for this PR.

I'm wondering if this should work such that for any action definition which says 'i can return a value' we let the user pick a local-variable name to write the value into.
move the state into the existing local variable system, instead of introducing a new flow for state that the user will need to understand behaves differently.

This would make it similar to how the generic-http module (and others?) do this today, but as they predate local-variables they can only write to custom variables. Perhaps it should be able to do both, but lets limit this to local to begin with.

I see pros and cons to each:

• This will be more tolerant to multiple concurrent executions, using local variables will not
• Local variables makes it obvious how things will flow around waits
• Sometimes, the user will want to fire off multiple requests in parallel and operate on the result of all of them, local variables solves the id/reference problem
• This is simpler to use in simple scenarios, no boilerplate of creating a local variable

$('Message').item.json.

I'm not opposed to something like this in the future, but I think what we use as the identifier needs some thought first. Something like this would be a start to addressing some other requests to reference/update properties of actions/feedbacks (some of which I am not sure is a good idea)

Why not use something like n8n does it: any node that's downstream, can access any upstream node's result using the node's name. For example, first node is called "Message", the next node can get the data directly with $json (in n8n) or $(this:result) with your implementation in Companion, but then next nodes or even the current one, can also get it with $('Message').item.json.

My concern here is that an action that sets a result, should not have that result remain live for unbounded period of time. (At least not unless a subsequent user of it chooses to keep it live.) Tying results directly and only to a bounded set of sequentially-next actions does this. Whereas if you could refer backwards to previous actions, perhaps even dynamically, then every action's result has to be kept alive "just in case".

There are plainly a bunch of ways to skin this cat. "Use it immediately or lose it" doesn't seem like too complicated a way to grasp, to me.

I'm wondering if this should work such that for any action definition which says 'i can return a value' we let the user pick a local-variable name to write the value into. move the state into the existing local variable system, instead of introducing a new flow for state that the user will need to understand behaves differently.

That makes some idiomatic sense.

I see pros and cons to each:

* This will be more tolerant to multiple concurrent executions, using local variables will not

Yeah. (If I were to grump on terminology, I'd name local variables as "per-entity variables", or better non-internal idiom terminology. And then maybe have some kind of truly local variable that can be created by an internal action, set from expression [or with this enhancement from an action result], that remains live only for the duration of the sequential action group it's found in. And would not have concurrency considerations to think about. But I digress.)

I guess the concurrency of local variables is my biggest quirk/beef here. Same as, but at least significantly reduced in troublesomeness, of a module variable, or of writing to a custom variable.

* Local variables makes it obvious how things will flow around waits

I think you could arguably say that implicitly piping the result distributing it across a concurrent action group is...of uncertain intuitiveness, as mechanism to enable multiple distinct manners of consuming an action result. So possibly the root problem isn't really the waits.

(I don't fully understand why waits in concurrent action groups are even given special meaning. Why not just require users to create a sequential action group, then interleave waits between concurrent action groups? But I digress even harder...)

* Sometimes, the user will want to fire off multiple requests in parallel and operate on the result of all of them, local variables solves the id/reference problem

* This is simpler to use in simple scenarios, no boilerplate of creating a local variable

Forcing stuff to be named doesn't feel like boilerplate to me. What does feel like boilerplate, is having to create/name them out of the actual UI of the step flow being executed. And having that variable exist across all steps, rather than only narrowly in the exact set of actions the computation/initialization happens in.